fix(settings): keep the settings modal open when a dropdown is dismissed - #1820
Conversation
Opening a Select inside the settings modal makes Radix set `pointer-events: none` on the dialog content, so the click that dismisses the dropdown falls through to the dialog's own overlay. Radix defers the dialog's outside-click decision to a `click` listener that runs after the Select has closed and handed pointer events back, so it no longer sees a layer above and dismisses the whole modal. SidebarModal never had the guard that ui/dialog.tsx already carried for this. Extract that guard into a shared `useDismissGuard` hook, have it also read the content's own inert state (the exact condition, rather than a proxy for it), and apply it to SidebarModal and CommandSearch — the two remaining raw Dialog.Content surfaces that host a popper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t shim Match the ui/ hook file naming convention (useSettingsLayout, useToast) and remove the globalThis.React assignment from the test — the module has no JSX, and the suite passes without it.
gabrielste1n
left a comment
There was a problem hiding this comment.
Verified end-to-end: reproduced the root-cause analysis against the Radix layer machinery, confirmed all four raw DialogPrimitive.Content sites are correctly triaged (SidebarModal fixed, CommandSearch's latent case fixed, ReferralModal correctly untouched, dialog.tsx behaviour-preserving refactor), and checked the guard can't over-block — Radix writes pointer-events: auto inline on a topmost modal's content, so the new signal only ever fires when a layer genuinely sits above. Bonus: it also fixes AlertDialog-over-Settings taking the whole modal down.
Tests pass 6/6, lint clean, typecheck failures identical to main (pre-existing missing deps). The backdrop-click-while-dropdown-open change to layered dismissal matches Escape behaviour and macOS convention — good call documenting it in the matrix.
Pushed one tidy-up commit: renamed dismissGuard.ts → useDismissGuard.ts to match the ui/ hook naming convention (useSettingsLayout, useToast) and dropped the globalThis.React test shim (the module has no JSX; suite passes without it). Excellent, thorough PR — thank you!
Problem — Opening a dropdown in Settings and then clicking anywhere in the panel closes the entire settings modal, not just the dropdown.
Fix — A dropdown makes the settings panel inert, so the dismissing click lands on the modal's backdrop. Settings never had the guard our other dialogs already carry against that; this adds it.
Problem
Reported with a screen recording. Sampling it frame-by-frame, the settings modal closes unexpectedly at 0:09, 0:24.5, 0:31 and 0:47 — every time, a dropdown is open and the next click anywhere inside the panel takes the whole modal down.
The three dropdowns involved are all our shared Radix
Select:MicrophoneSettingsMicrophoneSettingsSettingsPageThe native
<select>s in the same panel (Start position, Audio Retention, Transcript Retention) and theLanguageSelectorpickers are opened in the same recording and do not reproduce it — which is what pointed at the layer machinery rather than at any one control.Live probe against the real components in a browser (Playwright, script at the bottom):
Root cause
The dialog dismisses itself because the click it evaluates is, by then, indistinguishable from a click on its own backdrop:
Selectopening declaresdisableOutsidePointerEvents, so Radix setspointer-events: noneon the dialog's content — the settings panel goes inert (line 1 of the probe).Overlay, a full-viewportfixed inset-0scrim that Radix registers as the dialog's own dismiss affordance (line 2).DialogContentImplpassesdeferPointerDownOutside: true(@radix-ui/react-dialog@1.1.18), so the dialog's outside-click decision is deferred to a one-time documentclicklistener rather than settled atpointerdown.Selecthas closed and handed pointer events back. The gateif (!isPointerEventsEnabled …) returnin@radix-ui/react-dismissable-layerno longer sees a layer above, so it lets the dismissal through and closes the modal.The native
<select>s andLanguageSelectorare unaffected because neither is a Radix dismissable layer — the panel is never made inert, so their clicks land on real elements.src/components/ui/dialog.tsxalready carried a guard for this class (snapshot "was something above me" at pointerdown capture time).SidebarModal— the component Settings actually renders through — never got it.Fix
Extracted that guard into
src/components/ui/dismissGuard.tsasuseDismissGuard, and applied it where it was missing.The shared predicate keeps the two signals
dialog.tsxalready used (a mounted popper wrapper; a dialog stacked later in DOM order) and adds a third: the content's ownpointer-events: none. That is the exact condition that misroutes the click, rather than a proxy for it, and it also covers layers that render without a popper wrapper. It can only ever suppress a dismissal that was already wrong — a modal dialog with nothing above it carriespointer-events: auto, nevernone.Applied to:
SidebarModal— the settings modal, the reported bug.dialog.tsx— behaviour unchanged, now sharing one implementation instead of an inline copy.CommandSearch— the same latent defect: a rawDialog.Contenthosting aDropdownMenu. Not in the recording; flagged rather than left silently broken. Happy to split it out if you would rather keep this PR to Settings.Deliberately untouched
<select>s (Start position, Audio/Transcript Retention) — not dismissable layers, verified unaffected in the recording.LanguageSelector— portals into the closest[role="dialog"], so its clicks already count as inside. Unaffected.ReferralModal— the third rawDialog.Content, but it hosts no popper, so there is no bug to fix there.Tests
test/components/dismissGuard.test.js— 6 cases over the extracted policy, written first and watched fail with the module absent (its state onmain):then green after the fix (
pass 6, fail 0).Symptom-level RED → GREEN on this branch, toggling only the
SidebarModalwiring:Behaviour matrix, all green, confirming nothing else moved:
npm test: 2947 tests, 2758 pass, 16 fail, 172 skipped. The 16 failures are the pre-existingVoicePillsuite failing on an uninstalledborder-beamdependency — the identical 16 fail on a cleanorigin/maincheckout, andtscreports the same module as missing there too.npm run lintclean;npm run typecheckclean apart from those same pre-existing missing-dependency errors.Browser probe used above (needs a Playwright install; not committed — the repo has no browser-test harness)
Add
src/repro.html+src/repro.tsxmountingSidebarModalwith oneSelectinside, runcd src && npx vite, then:Provenance
Screen recording from Josh, 2026-08-25: https://cap.so/s/h79j2y5m2k4yvv9